Skip to content

regression: room list not using full sidebar width in new navigation #35454

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
kodiakhq merged 6 commits into from
Mar 11, 2025
Merged

regression: room list not using full sidebar width in new navigation #35454

kodiakhq merged 6 commits into from
Mar 11, 2025

Conversation

@abhinavkrin
Copy link
Member

@abhinavkrin abhinavkrin commented Mar 10, 2025 •
edited by dougfabris
Loading

Proposed changes (including videos or screenshots)

This PR fixes an issue where the room list in the new navigation was not occupying full of the sidebar width.

Introduced here: #33796

Before
Screenshot 2025-03-11 at 2 11 45 AM

After
Occupies full width

Issue(s)

Steps to test or reproduce

  1. Enable feature preview
  2. Enable enhanced navigation
  3. Verify that the room list now takes up the full width of the sidebar.

Further comments

CORE-1016

@abhinavkrin
fix: RoomsList occupies half width of sidebar
00215f8 
Signed-off-by: Abhinav Kumar <[email protected]>
@abhinavkrin
added tests
45a08ab 
Signed-off-by: Abhinav Kumar <[email protected]>
@abhinavkrin abhinavkrin requested a review from a team as a code owner March 10, 2025 20:50
@dionisio-bot
Copy link
Contributor

dionisio-bot bot commented Mar 10, 2025 •
edited
Loading

Looks like this PR is ready to merge! 🎉
If you have any trouble, please check the PR guidelines

@changeset-bot
Copy link

changeset-bot bot commented Mar 10, 2025 •
edited
Loading

⚠️ No Changeset found

Latest commit: f49d285

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@abhinavkrin
Copy link
Member Author

abhinavkrin commented Mar 10, 2025

I have not added changeset, since the issue is not present in the previous version.

@abhinavkrin abhinavkrin added this to the 7.5.0 milestone Mar 10, 2025
@github-actions
Copy link
Contributor

github-actions bot commented Mar 10, 2025

PR Preview Action v1.6.0

🚀 View preview at
https://RocketChat.github.io/Rocket.Chat/pr-preview/pr-35454/

Built to branch gh-pages at 2025-03-10 20:57 UTC.
Preview will be ready when the GitHub Pages deployment is complete.

@codecov
Copy link

codecov bot commented Mar 10, 2025 •
edited
Loading

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 59.68%. Comparing base (05fad33) to head (f49d285).
Report is 1 commits behind head on develop.

Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff             @@
##           develop   #35454      +/-   ##
===========================================
+ Coverage    59.64%   59.68%   +0.03%     
===========================================
  Files         2828     2829       +1     
  Lines        68542    68790     +248     
  Branches     15193    15238      +45     
===========================================
+ Hits         40884    41056     +172     
- Misses       25010    25075      +65     
- Partials      2648     2659      +11
Flag Coverage Δ
unit 75.87% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

🚀 New features to boost your workflow:
  • ❄ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

dougfabris
dougfabris requested changes Mar 10, 2025
View reviewed changes
Copy link
Member

@dougfabris dougfabris left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Its not a fix since this bug wasn't shipped in any version. Let's use regression for that!

@abhinavkrin abhinavkrin changed the title fix: room list not using full sidebar width in new navigation regression: room list not using full sidebar width in new navigation Mar 11, 2025
@abhinavkrin
removed test
6fb8d24 
Signed-off-by: Abhinav Kumar <[email protected]>
@abhinavkrin
requested changes
c930349 
Signed-off-by: Abhinav Kumar <[email protected]>
@abhinavkrin abhinavkrin requested a review from dougfabris March 11, 2025 10:20
@abhinavkrin
requested changes
3476de0 
Signed-off-by: Abhinav Kumar <[email protected]>
dougfabris
dougfabris approved these changes Mar 11, 2025
View reviewed changes
Copy link
Member

@dougfabris dougfabris left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the very quick fix 👍

@dougfabris dougfabris added the stat: QA assured Means it has been tested and approved by a company insider label Mar 11, 2025
@dionisio-bot dionisio-bot bot added the stat: ready to merge PR tested and approved waiting for merge label Mar 11, 2025
@kodiakhq kodiakhq bot merged commit 43a4cee into develop Mar 11, 2025
48 checks passed
@kodiakhq kodiakhq bot deleted the fix/room-list-width-half-of-sidebarV2 branch March 11, 2025 13:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@dougfabris dougfabris dougfabris approved these changes

Assignees

No one assigned

Labels

stat: QA assured Means it has been tested and approved by a company insider stat: ready to merge PR tested and approved waiting for merge

Projects

None yet

Milestone

7.5.0

Development

Successfully merging this pull request may close these issues.

3 participants

@abhinavkrin @dougfabris